Skip to content

guard seat_get_focus() NULL dereferences#9055

Open
hack-heart wants to merge 2 commits into
swaywm:masterfrom
hack-heart:sway-fix-seat-get-focus-null-deref
Open

guard seat_get_focus() NULL dereferences#9055
hack-heart wants to merge 2 commits into
swaywm:masterfrom
hack-heart:sway-fix-seat-get-focus-null-deref

Conversation

@hack-heart
Copy link
Copy Markdown

@hack-heart hack-heart commented Mar 1, 2026

seat_get_focus() can return NULL when the focus stack is empty. a few call sites don't check for this and just dereference the result, which crashes sway.

this happens when a client disconnects abruptly, leaving the seat with a stale has_focus flag or empty focus stack. whatever touches focus next dereferences NULL. the two confirmed crash sites were seat_set_workspace_focus and seat_set_focus_surface (coredumps below). seat_unfocus_unless_client has the same pattern so a check was added there too for safety.

Coredumps:

I cannot remember how i triggered this one:

#0  seat_send_unfocus ()
#1  seat_set_workspace_focus ()
#2  seat_set_focus ()
#3  wl_signal_emit_mutable ()
#4  container_begin_destroy ()
#5  view_unmap ()
#6  handle_unmap ()
#7  wl_signal_emit_mutable ()
#8  wlr_surface_unmap ()
#9  destroy_xdg_toplevel ()
#10 destroy_xdg_surface_role_object ()
#11 destroy_xdg_surface ()
#12 xdg_client_handle_resource_destroy ()
#13 wl_client_destroy ()

Mouse click on empty container (clicked wallpaper, likely was just after a wallpaper engine crash or something):

#0  seat_send_unfocus ()
#1  seat_set_workspace_focus ()
#2  seat_set_focus ()
#3  handle_button ()
#4  wl_signal_emit_mutable ()
#5  handle_pointer_button ()
#6  handle_libinput_readable ()
#7  wl_event_loop_dispatch ()

IPC workspace switch (with noctalia workspace widget):

#0  seat_send_unfocus ()
#1  seat_set_workspace_focus ()
#2  seat_set_focus ()
#3  workspace_switch ()
#4  cmd_workspace ()
#5  execute_command ()
#6  ipc_client_handle_command ()
#7  ipc_client_handle_readable ()
#8  wl_event_loop_dispatch ()

Layer surface teardown (restarted noctalia shell):

#0  seat_send_unfocus ()
#1  seat_set_focus_surface ()
#2  seat_set_focus_layer ()
#3  handle_node_destroy ()
#4  wl_signal_emit_mutable ()
#5  sway_scene_node_destroy ()
#6  layer_surface_destroy ()
#7  surface_handle_role_resource_destroy ()

i encountered this while using scroll and initially opened a pr against it dawsers/scroll#219

seat_get_focus() can return NULL when the focus stack is empty. a few
call sites don't check for this and just dereference the result, which
crashes sway.

this happens when a client disconnects abruptly, leaving the seat with
a stale has_focus flag or empty focus stack. whatever touches focus next
dereferences NULL. the two confirmed crash sites were
seat_set_workspace_focus and seat_set_focus_surface (coredumps below).
seat_unfocus_unless_client has the same pattern so a check was added
there too for safety.

cannot remember how this one was triggered:

```
seat_send_unfocus ()
seat_set_workspace_focus ()
seat_set_focus ()
wl_signal_emit_mutable ()
container_begin_destroy ()
view_unmap ()
handle_unmap ()
wl_signal_emit_mutable ()
wlr_surface_unmap ()
destroy_xdg_toplevel ()
destroy_xdg_surface_role_object ()
destroy_xdg_surface ()
xdg_client_handle_resource_destroy ()
wl_client_destroy ()
```

mouse click on empty container (clicked wallpaper, likely just after a
wallpaper engine crash):

```
seat_send_unfocus ()
seat_set_workspace_focus ()
seat_set_focus ()
handle_button ()
wl_signal_emit_mutable ()
handle_pointer_button ()
handle_libinput_readable ()
wl_event_loop_dispatch ()
```

IPC workspace switch (with noctalia workspace widget):

```
seat_send_unfocus ()
seat_set_workspace_focus ()
seat_set_focus ()
workspace_switch ()
cmd_workspace ()
execute_command ()
ipc_client_handle_command ()
ipc_client_handle_readable ()
wl_event_loop_dispatch ()
```

layer surface teardown (restarted noctalia shell):

```
seat_send_unfocus ()
seat_set_focus_surface ()
seat_set_focus_layer ()
handle_node_destroy ()
wl_signal_emit_mutable ()
sway_scene_node_destroy ()
layer_surface_destroy ()
surface_handle_role_resource_destroy ()
```
@hack-heart hack-heart force-pushed the sway-fix-seat-get-focus-null-deref branch from 123746a to 7ef26f9 Compare March 1, 2026 10:30
@emersion
Copy link
Copy Markdown
Member

emersion commented Mar 5, 2026

Shouldn't we set seat->has_focus to false when setting the focus to NULL?

Set has_focus to false in seat_unfocus_unless_client when seat_get_focus() returns NULL.
@hack-heart
Copy link
Copy Markdown
Author

just added that

@hack-heart
Copy link
Copy Markdown
Author

@emersion did you get a chance to re-review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants